Introduce follower replication by Fullstop000 · Pull Request #249 · tikv/raft-rs

Fullstop000 · 2019-07-01T08:31:23Z

#136

New features

Add some new fields in Message
New concepts Group and Delegate in the cluster topology
~~A message now could be sent to a proxy and the proxy redirects the message to the destination~~

TODOs

The leader can specify the Delegate of a Group
The Delegate can bcast_append or send_append to the other members in the Group
Stable group delegate
Scenarios based testing

Problems

~~The ConfChange protocol for raft groups might need to be discussed.~~ Solved as GroupConfig is now volatile.

The implementation has been ported to etcd: etcd-io/etcd#11455.

Fullstop000 · 2019-09-05T02:24:07Z

I've finished the main part of this PR. But I'm not sure the best way to implement the ConfChange for altering raft groups dynamically so I want a discussion with your guys.

PTAL @siddontang @hicqu @BusyJay @Hoverbear

Hoverbear · 2019-09-09T20:38:17Z

@Fullstop000 Can you please include links to any design docs you worked on?

Fullstop000 · 2019-09-10T06:22:57Z

@Hoverbear Can I make a WIP PR in tikv/rfc ？

Hoverbear · 2019-09-18T21:45:02Z

@Fullstop000 I think it's OK. :) I'd still really appreciate seeing an RFC (like https://docs.google.com/document/d/1Sp9Tnc_nk_i0feTOgLGPT3jZYVFjfP7C6pwA-wuVHko/edit?ts=5c8c5fe3 which you worked on)

Hoverbear · 2019-11-07T19:36:38Z

@hicqu PTAL

hicqu · 2019-12-03T12:29:37Z

+
+    /// The max number of members of a group of which contains leader and the leader never pick a delegate.
+    /// If the group size is larger than this, a delegate will be picked even the leader belongs to this group.
+    pub max_leader_group_no_delegate: usize,


Personally I prefer to remove this, because without explicit information about group_id, follower replication may be not helpful. For example 7 peers in one datacenter, follower replication can't save any network traffics.

hicqu · 2019-12-03T12:37:21Z

+    // Also, A peer with smaller 'match' is able to receive more un-compacted entries from leader
+    // and then send them to others in same group.
+    //
+    fn pick_delegate(&self, group: &[u64], prs: &ProgressSet) -> u64 {


Seems in the new implementation, choose the peer with most raft logs is better?

It seems more reasonable. Actually maybe we can choose any unpaused peer?

hicqu · 2019-12-04T11:43:24Z

    pub msgs: Vec<Message>,

+    /// The especial map for messages to be sent to group delegates.
+    pub delegated_msgs: HashMap<u64, Message>,


I prefer to put it with groups

hicqu · 2019-12-04T11:56:58Z

    }

+    // Whether the given peer could use Follower Replication
+    fn use_delegate(&self, to: u64) -> bool {


I prefer

fn use_delegate(&self, to: u64) -> bool { self.is_leader() && self.groups.get_members(to).map_or(0, |members| members.len()) > 1 && !self.groups.in_same_group(self.id, to) }

And we need to make sure that peers with invalid group_id shouldn't exist in groups.

hicqu · 2019-12-04T11:58:49Z

+    /// of produced message should be set to the leader id.
+    pub fn send_append(&mut self, to: u64, prs: &mut ProgressSet) {
+        if self.use_delegate(to) {
+            if let Some(gid) = self.groups.get_group_id(to) {


Too many indents. Maybe we can define use_delegate(to) -> Option<group_id>.

hicqu · 2019-12-04T12:02:24Z

+                            None => self.pick_delegate(&members, prs),
+                        };
+                        if delegate != INVALID_ID {
+                            if let Some(pr) = prs.get_mut(delegate) {


I think calling unwrap is better, because the progress must exist.

hicqu · 2019-12-04T12:08:04Z

+                    }
+                    None => {
+                        let members = self.groups.get_members(to).cloned().unwrap(); // this is safe because of the checking in `self.use_delegate`
+                        let delegate = match self.groups.get_delegate(gid) {


I prefer let delegate = self.pick_delegate(prs), which already contains get_delegate logic. And, its valid only if the pr is not paused.

hicqu · 2019-12-04T12:17:14Z

        *maybe_commit = true;
    }

+    fn handle_append_response_in_delegate(


Seems only difference between this and handle_append_response is the latter needs to handle leader transferring. That logic is compatible for delegates. So I think this function is unnecessary.

hicqu · 2019-12-04T12:29:54Z

+                self.set_prs(prs);
+            } else {
+                let members = self.groups.get_members(m.from_delegate).cloned();
+                self.bcast_append(members.as_ref());


bcast_append calls send_append, which checks members' delegate again. I believe it can be improved, maybe we can take a try?

hicqu · 2019-12-04T12:35:30Z

                self.read_states.push(rs);
            }
+            MessageType::MsgAppendResponse => {
+                if self.prs().get(m.from).is_none() {


It's a common case for receiving responses from removed peers. So I think the log is unnecessary. Without it the code could be cleaner.

hicqu · 2019-12-04T12:42:00Z

+    //
+    // The delegate must satisfy conditions below:
+    // 1. Must be 'recent_active'
+    // 2. The progress state should be 'Replicate' but not 'paused'


Seems Probe needs also to be allowed? I think calling is_paused here is fine.

Probe might not be suitable for a delegate in the leader's view because the Probe member can probably just recover from a crash or is suffering the network partition.

hicqu · 2019-12-05T11:15:53Z

+    }
+
+    #[inline]
+    fn is_follower_replication_enabled(&self) -> bool {


I think it's not accurate. Every peer can't know follower replication is enabled or not. It just choose a strategy after receives followers' messages.

We can add some comments to make this clear.

Signed-off-by: Fullstop000 <fullstop1005@gmail.com>

TennyZhuang · 2021-08-20T08:44:20Z

Any updates?

Xuanwo · 2021-08-24T05:49:51Z

This PR is discontinued for so long, any updates here? Do we have a plan to get it merged?

Fullstop000 changed the title ~~[WIP] Introduce follower replication~~ Introduce follower replication Aug 27, 2019

Hoverbear requested review from BusyJay, Hoverbear and hicqu September 9, 2019 19:00

Fullstop000 force-pushed the follower_replication branch 4 times, most recently from ae47fee to 3502119 Compare October 29, 2019 09:45

Hoverbear assigned Fullstop000 Nov 7, 2019

Hoverbear added the Feature Related to a major feature. label Nov 7, 2019

Fullstop000 mentioned this pull request Nov 14, 2019

Introduce Follower Replication tikv/rfcs#33

Open

Fullstop000 force-pushed the follower_replication branch from 3502119 to 0902e2f Compare November 19, 2019 03:00

hicqu mentioned this pull request Nov 25, 2019

Incubating Program: Follower Read With Applied Index pingcap/community#86

Closed

9 tasks

hicqu reviewed Dec 2, 2019

View reviewed changes

Comment thread src/raft_log.rs

hicqu reviewed Dec 2, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 2, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 2, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 3, 2019

View reviewed changes

Comment thread src/group/mod.rs Outdated

hicqu reviewed Dec 3, 2019

View reviewed changes

Comment thread src/group/mod.rs Outdated

hicqu reviewed Dec 3, 2019

View reviewed changes

hicqu reviewed Dec 4, 2019

View reviewed changes

hicqu reviewed Dec 5, 2019

View reviewed changes

Hoverbear reviewed Dec 6, 2019

View reviewed changes

Comment thread harness/src/interface.rs Outdated

Hoverbear reviewed Dec 6, 2019

View reviewed changes

Comment thread harness/src/network.rs

Hoverbear reviewed Dec 6, 2019

View reviewed changes

Comment thread harness/tests/integration_cases/test_raft_follower_replication.rs Outdated

hicqu reviewed Dec 11, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 11, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 11, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 11, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 11, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 11, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 11, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 11, 2019

View reviewed changes

Comment thread src/raft.rs Outdated

hicqu reviewed Dec 17, 2019

View reviewed changes

Comment thread src/progress/progress_set.rs Outdated

Fullstop000 force-pushed the follower_replication branch from 4259f29 to 8ed4829 Compare December 17, 2019 07:25

hicqu previously approved these changes Dec 17, 2019

View reviewed changes

Fullstop000 dismissed hicqu’s stale review via 5189128 January 19, 2020 08:13

Fullstop000 force-pushed the follower_replication branch from b7d2176 to 5189128 Compare January 19, 2020 08:13

Fullstop000 force-pushed the follower_replication branch 2 times, most recently from 88f583e to f89086c Compare February 25, 2020 08:17

squash commits

1da637c

Signed-off-by: Fullstop000 <fullstop1005@gmail.com>

Fullstop000 force-pushed the follower_replication branch 2 times, most recently from 88f583e to 1da637c Compare February 25, 2020 08:31

make clippy happy

f1592d3

Signed-off-by: Fullstop000 <fullstop1005@gmail.com>

Conversation

Fullstop000 commented Jul 1, 2019 • edited by hicqu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New features

TODOs

Problems

Uh oh!

Fullstop000 commented Sep 5, 2019

Uh oh!

Hoverbear commented Sep 9, 2019

Uh oh!

Fullstop000 commented Sep 10, 2019

Uh oh!

Hoverbear commented Sep 18, 2019

Uh oh!

Hoverbear commented Nov 7, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hicqu Dec 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TennyZhuang commented Aug 20, 2021

Uh oh!

Xuanwo commented Aug 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Fullstop000 commented Jul 1, 2019 •

edited by hicqu

Loading

hicqu Dec 4, 2019 •

edited

Loading